fix(baseball): CoachHelm engine Phase 4b — canonical stat reads + deterministic import-quality clock (#379, #811 residual) - #852
Closed
njrini99-code wants to merge 1 commit into
Closed
Conversation
…weep/baseline/engine-run + deterministic import-quality clock (#379, #811 residual) Reader migration (#379 Phase 4b, the highest-blast-radius chunk): - NEW src/lib/baseball/coachhelm/engine-stat-rows.ts — the ONE consolidated per-session stat-row read for the engine. Prefers canonical baseball_box_score_batting/_pitching rows (normalized onto the loader shape via #851's normalizeBoxScoreBattingRow/normalizeBoxScorePitchingRow, with session_date joined from baseball_games and source-table provenance tags), reconciled over legacy baseball_player_stats rows per the #379 precedence rule: canonical rows replace a player's legacy GAME rows outright (never blended — the #827 seed writes the same games into both layers), legacy practice/other rows always survive (practice carve-out), and a player with zero canonical rows keeps full legacy history (fallback tier). Canonical-side read failures degrade all-or-nothing to the legacy pool; a legacy read failure remains the callers' hard error. All reads paginate past the PostgREST 1000-row cap with stable ordering. - outcome-sweep.ts / action-baseline.ts / engine-run.ts all swap their direct baseball_player_stats reads for the shared helper, so baseline capture, the outcome sweep, and the engine run measure the SAME reconciled pool (apples-to-apples did-it-move). action-baseline's old single-page .limit(1000) read is replaced by the paginated shared read. #811 residual (deterministic engine clock): - BaseballV10EngineInputs gains an optional now (ISO); engine-run threads its nowIso through it; importQualityGenerator's 14-day recency window computes from the caller-supplied nowIso instead of raw Date.now() (default preserves real-time behavior for non-engine callers). New generators/v10.test.ts pins the window against a fixed 2020 clock; engine-run-helm-lifting.test.ts pins that runBaseballEngineCore passes its own nowIso end-to-end. Provenance labels: - generators/index.ts driver() last-resort fallback label no longer hardcodes the deprecated table (loaders now cite the real per-row table); v10.ts's practice-effectiveness cite stays deliberately (its feeder still reads legacy practice rows) with an explanatory comment. Manifest (stat-layer contract kept green in both directions for this chunk): - Removed migrated entries: outcome-sweep.ts, engine-run.ts, action-baseline.ts, generators/index.ts, ai-policy-enforcement.test.ts, signal-from-insight.test.ts (fixtures moved to canonical table names). - Added: engine-stat-rows.ts + its test (the one allowed legacy-fallback read). - Updated notes: loaders.ts, generators/v10.ts, effectiveness/engine.ts and operational-rule-engine.ts (both reviewed, deliberately deferred — their cites are honest while their feeders still read layer 1), plus the three engine test entries now pinning the fallback tier. Tests: engine-stat-rows.test.ts pins the precedence rule directly; action-baseline.test.ts + outcome-sweep-insight-resolve.test.ts gain canonical-preferred, never-blended coverage alongside the existing legacy-fallback pins. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR introduces
Confidence Score: 4/5Safe to merge. The three-tier precedence rule is pinned by seven focused unit tests across two new test files and three extended ones; no hard rules are violated. Both findings are documented design decisions — neither introduces wrong data nor a regression. Tables are sport-prefixed, no service-role leakage, no destructive writes, no client-side LLM calls. The engine-stat-rows.ts lines 179–181: the canonical fallback path silently returns error:null; worth adding a log or counter before this path goes to high-traffic production load. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[engine-run.ts / outcome-sweep.ts / action-baseline.ts] -->|teamId, playerIds| B[loadEngineStatRows]
B --> C[Promise.all]
C --> D[baseball_player_stats legacy]
C --> E[baseball_games all team]
C --> F[baseball_box_score_batting player-filtered]
C --> G[baseball_box_score_pitching player-filtered]
D --> H{Legacy read failed?}
H -->|yes| I[data:null error]
H -->|no| J{Any canonical read failed?}
E --> J
F --> J
G --> J
J -->|yes| K[data:legacyRows error:null silent fallback]
J -->|no| L{Canonical rows exist?}
L -->|none| M[data:legacyRows error:null rule 3]
L -->|some| N[normalize batting + pitching rows]
N --> O[Drop legacy game rows for box-score players rule 1]
O --> P[Keep practice rows for all players rule 2]
P --> Q[normalized + retainedLegacy BoxScoreRow pool]
Q --> R[loadAllPlayerMetrics / metrics registry]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[engine-run.ts / outcome-sweep.ts / action-baseline.ts] -->|teamId, playerIds| B[loadEngineStatRows]
B --> C[Promise.all]
C --> D[baseball_player_stats legacy]
C --> E[baseball_games all team]
C --> F[baseball_box_score_batting player-filtered]
C --> G[baseball_box_score_pitching player-filtered]
D --> H{Legacy read failed?}
H -->|yes| I[data:null error]
H -->|no| J{Any canonical read failed?}
E --> J
F --> J
G --> J
J -->|yes| K[data:legacyRows error:null silent fallback]
J -->|no| L{Canonical rows exist?}
L -->|none| M[data:legacyRows error:null rule 3]
L -->|some| N[normalize batting + pitching rows]
N --> O[Drop legacy game rows for box-score players rule 1]
O --> P[Keep practice rows for all players rule 2]
P --> Q[normalized + retainedLegacy BoxScoreRow pool]
Q --> R[loadAllPlayerMetrics / metrics registry]
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
src/lib/baseball/coachhelm/engine-stat-rows.ts:179-181
**Silent canonical fallback — no observability signal**
When any canonical read fails, the function returns `{ data: legacyRows, error: null }`. All three callers receive `error: null` and cannot distinguish a successful canonical read from a silent degrade to legacy. If `baseball_box_score_batting` or `baseball_box_score_pitching` develops an RLS denial or transient error in production, the engine continues generating insights from legacy data indefinitely with no alert, Datadog counter, or ledger entry. A structured log line or a `canonicalFallback: boolean` field in the return shape would let the team detect the condition, consistent with the effectiveness ledger's observability philosophy.
### Issue 2 of 2
src/lib/baseball/coachhelm/engine-stat-rows.ts:144-156
**`baseball_games` query is team-wide, not scoped to the requested players**
The games query fetches every `(id, game_date)` pair for the entire team regardless of which `playerIds` were requested. For `action-baseline.ts`—which calls this with a single `playerId`—a full season of team games is fetched to resolve dates for perhaps a handful of batting rows. The parallel `Promise.all` design prevents a two-phase approach without sequential round-trips, so the trade-off is intentional; a future optimization could apply a `WHERE id IN (game_ids_from_batting_pitching)` filter after the parallel fetch phase to reduce payload from O(team-season) to O(player-game-count).
Reviews (1): Last reviewed commit: "fix(baseball): CoachHelm engine Phase 4b..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+179
to
+181
| if (gamesRes.error || battingRes.error || pitchingRes.error) { | ||
| return { data: legacyRows, error: null }; | ||
| } |
There was a problem hiding this comment.
When any canonical read fails, the function returns { data: legacyRows, error: null }. All three callers receive error: null and cannot distinguish a successful canonical read from a silent degrade to legacy. If baseball_box_score_batting or baseball_box_score_pitching develops an RLS denial or transient error in production, the engine continues generating insights from legacy data indefinitely with no alert, Datadog counter, or ledger entry. A structured log line or a canonicalFallback: boolean field in the return shape would let the team detect the condition, consistent with the effectiveness ledger's observability philosophy.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/baseball/coachhelm/engine-stat-rows.ts
Line: 179-181
Comment:
**Silent canonical fallback — no observability signal**
When any canonical read fails, the function returns `{ data: legacyRows, error: null }`. All three callers receive `error: null` and cannot distinguish a successful canonical read from a silent degrade to legacy. If `baseball_box_score_batting` or `baseball_box_score_pitching` develops an RLS denial or transient error in production, the engine continues generating insights from legacy data indefinitely with no alert, Datadog counter, or ledger entry. A structured log line or a `canonicalFallback: boolean` field in the return shape would let the team detect the condition, consistent with the effectiveness ledger's observability philosophy.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+144
to
+156
| fetchAllRowsResult<GameRow>((from, to) => | ||
| db | ||
| .from('baseball_games') | ||
| .select('id, game_date') | ||
| .eq('team_id', teamId) | ||
| .order('id', { ascending: true }) | ||
| .range(from, to), | ||
| ), | ||
| fetchAllRowsResult<BoxBattingRow>((from, to) => | ||
| db | ||
| .from('baseball_box_score_batting') | ||
| .select(BOX_BATTING_SELECT) | ||
| .eq('team_id', teamId) |
There was a problem hiding this comment.
baseball_games query is team-wide, not scoped to the requested players
The games query fetches every (id, game_date) pair for the entire team regardless of which playerIds were requested. For action-baseline.ts—which calls this with a single playerId—a full season of team games is fetched to resolve dates for perhaps a handful of batting rows. The parallel Promise.all design prevents a two-phase approach without sequential round-trips, so the trade-off is intentional; a future optimization could apply a WHERE id IN (game_ids_from_batting_pitching) filter after the parallel fetch phase to reduce payload from O(team-season) to O(player-game-count).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/baseball/coachhelm/engine-stat-rows.ts
Line: 144-156
Comment:
**`baseball_games` query is team-wide, not scoped to the requested players**
The games query fetches every `(id, game_date)` pair for the entire team regardless of which `playerIds` were requested. For `action-baseline.ts`—which calls this with a single `playerId`—a full season of team games is fetched to resolve dates for perhaps a handful of batting rows. The parallel `Promise.all` design prevents a two-phase approach without sequential round-trips, so the trade-off is intentional; a future optimization could apply a `WHERE id IN (game_ids_from_batting_pitching)` filter after the parallel fetch phase to reduce payload from O(team-season) to O(player-game-count).
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Owner
Author
|
Superseded by the rebased + review-fixed branch task/379-engine-4b-v2 (see the new PR) — closed to avoid a force-push over the reviewed history. |
njrini99-code
added a commit
that referenced
this pull request
Jul 15, 2026
…scoped precedence (supersedes #852) (#854) * fix(baseball): CoachHelm engine Phase 4b — canonical stat reads for sweep/baseline/engine-run + deterministic import-quality clock (#379, #811 residual) Reader migration (#379 Phase 4b, the highest-blast-radius chunk): - NEW src/lib/baseball/coachhelm/engine-stat-rows.ts — the ONE consolidated per-session stat-row read for the engine. Prefers canonical baseball_box_score_batting/_pitching rows (normalized onto the loader shape via #851's normalizeBoxScoreBattingRow/normalizeBoxScorePitchingRow, with session_date joined from baseball_games and source-table provenance tags), reconciled over legacy baseball_player_stats rows per the #379 precedence rule: canonical rows replace a player's legacy GAME rows outright (never blended — the #827 seed writes the same games into both layers), legacy practice/other rows always survive (practice carve-out), and a player with zero canonical rows keeps full legacy history (fallback tier). Canonical-side read failures degrade all-or-nothing to the legacy pool; a legacy read failure remains the callers' hard error. All reads paginate past the PostgREST 1000-row cap with stable ordering. - outcome-sweep.ts / action-baseline.ts / engine-run.ts all swap their direct baseball_player_stats reads for the shared helper, so baseline capture, the outcome sweep, and the engine run measure the SAME reconciled pool (apples-to-apples did-it-move). action-baseline's old single-page .limit(1000) read is replaced by the paginated shared read. #811 residual (deterministic engine clock): - BaseballV10EngineInputs gains an optional now (ISO); engine-run threads its nowIso through it; importQualityGenerator's 14-day recency window computes from the caller-supplied nowIso instead of raw Date.now() (default preserves real-time behavior for non-engine callers). New generators/v10.test.ts pins the window against a fixed 2020 clock; engine-run-helm-lifting.test.ts pins that runBaseballEngineCore passes its own nowIso end-to-end. Provenance labels: - generators/index.ts driver() last-resort fallback label no longer hardcodes the deprecated table (loaders now cite the real per-row table); v10.ts's practice-effectiveness cite stays deliberately (its feeder still reads legacy practice rows) with an explanatory comment. Manifest (stat-layer contract kept green in both directions for this chunk): - Removed migrated entries: outcome-sweep.ts, engine-run.ts, action-baseline.ts, generators/index.ts, ai-policy-enforcement.test.ts, signal-from-insight.test.ts (fixtures moved to canonical table names). - Added: engine-stat-rows.ts + its test (the one allowed legacy-fallback read). - Updated notes: loaders.ts, generators/v10.ts, effectiveness/engine.ts and operational-rule-engine.ts (both reviewed, deliberately deferred — their cites are honest while their feeders still read layer 1), plus the three engine test entries now pinning the fallback tier. Tests: engine-stat-rows.test.ts pins the precedence rule directly; action-baseline.test.ts + outcome-sweep-insight-resolve.test.ts gain canonical-preferred, never-blended coverage alongside the existing legacy-fallback pins. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): scope engine-stat-rows precedence to (player, date), not player alone (#379, #852 review fix) The #379 exclusion rule dropped ALL of a player's legacy stat_type='game' rows once they had ANY canonical box-score row, even for games with no canonical counterpart — a mid-season box-score import start would silently erase that player's earlier legacy-logged games from every engine caller (engine-run, outcome-sweep, action-baseline), shrinking sample_n and flipping confidence/verdicts. baseball_player_stats has no game_id, so we now correlate on the resolved canonical game date (truncated to YYYY-MM-DD on both sides) instead: a legacy game row is dropped only when that same player has canonical coverage on that exact calendar day: a same-day heuristic, not a guaranteed game-identity match, since there's no FK to lean on (documented in the module comment as an accepted double-header collision risk). Existing precedence tests encoded the bug: their legacy-row fixture dates never matched the canonical game dates, yet still asserted full drop — only possible under the old player-scoped exclusion. Realigned those fixture dates to same-day overlap (preserving each test's 100%-coverage intent) and added a mixed-coverage case (3 legacy-only + 2 canonical -> 5 rows survive) plus a same-day-collision case (legacy row on a canonically-covered date -> dropped). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa --------- Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
njrini99-code
added a commit
that referenced
this pull request
Jul 15, 2026
…-sweep/action-baseline (#852 residual) (#864) * baseball(engine): wire event-derived velocity into engine-run/outcome-sweep/action-baseline (#852 residual) Box-score-migrated players had NO velocity metrics: their legacy exit_velocity/pitch_velocity scalar is dropped alongside superseded legacy GAME rows (engine-stat-rows.ts rule 1), and the canonical box-score tables carry no velocity columns at all. loaders.ts's eventDerived hook (#851) already threaded a per-field event-layer override into loadPlayerMetrics, but nothing called it. Adds src/lib/baseball/coachhelm/engine-event-derived.ts: a team-scoped, paginated read of baseball_pitch_events/baseball_batted_ball_events (#813 superseded-row filter) plus a pure per-player reducer that reuses elite-stat-events.ts's real buildHitterMetrics/buildPitcherMetrics + loaders.ts's eventDerivedVelocityFromMetrics -- never a second, drifting "average exit velocity" implementation. All-or-nothing degrade on read failure, mirroring engine-stat-rows.ts's own honesty rule. Wires it into all three engine callers: - engine-run.ts: full-history event pool -> loadAllPlayerMetrics. - outcome-sweep.ts: event rows filtered to the SAME per-action after-window as the box-score read, so a pre-action event never counts toward did-it-move measurement. - action-baseline.ts: full-history event pool -> the baseline capture. Tests: pure aggregation (mixed hitter/pitcher, zero-event absence, supersede filter, all-or-nothing degrade) plus per-caller wiring tests (event wins over legacy scalar for the same player; a zero-event player keeps their legacy velocity; event-read failure degrades every player to legacy). Extends stat-layer-manifest.ts's grandfathered-consumer allowlist for the new fixture files (legacy baseball_player_stats rows are the fallback pin, not staleness). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): bound velocity event read to player scope + fix sampleSize honesty (PR #864 fix-first) Two adversarial-review criticals on #864: 1. buildActionOutcomeSeed (action-baseline.ts) fired a TEAM-WIDE, unbounded, player-unscoped read of the entire pitch/batted-ball event history on every coach "convert to action" click, just to resolve ONE player's velocity scalar. loadEngineEventRows now takes an optional `playerIds` scope (`.in('pitcher_id'|'batter_id', playerIds)`, mirroring loadEngineStatRows's own `.in('player_id', playerIds)` idiom) — the single-player caller passes `[playerId]`; engine-run/outcome-sweep now pass their own already-computed roster/todo player-id lists instead of reading the whole team's history. 2. avg_exit_velocity's sampleSize was `bbCount` (every batted ball) instead of the count of rows that actually carried a non-null exit_velocity reading — inflating the honesty gate for any team whose batted-ball capture doesn't always log a radar reading. Fixed to `battedBalls.filter(b => b.exit_velocity != null).length`, and applied the same fix to the sibling avg_launch_angle metric (identical bug, same line shape). Pitcher avg_velocity was already correct. Tests: pin the DB-level player scoping (loadEngineEventRows + a buildActionOutcomeSeed integration check), and pin the sampleSize fix (10 batted balls / 4 readings -> sampleSize 4; independent launch_angle gating; hard_hit_rate's bbCount-based denominator unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
njrini99-code
added a commit
that referenced
this pull request
Jul 16, 2026
…al-audit infra, de-vibe wave 2a, public-page motion fix (#868) * devibe: remove dead files — knip batch 1/2 (mode-toggle, notifications, insight-actions) (#858) Verified dead via grep (import path + symbol name + next/dynamic scan), then git rm. No consumers found in src/, no test coverage, no dynamic imports referencing any of these paths. - src/components/baseball/coach/ModeToggle.tsx — exports JUCOModeToggle, zero importers repo-wide. Only referenced from stale docs (PHASE_5_JUCO_COACH.md, .helm/ACTIONS.md) describing a wiring into src/components/layout/header.tsx, which no longer exists. - src/components/layout/mode-toggle.tsx — exports ModeToggle/Mode, its only consumer was the dead file above. - src/components/features/notification-center.tsx — duplicate/legacy NotificationCenter; the live one is src/components/golf/calendar/NotificationCenter.tsx. .taskmaster/docs/current-state.md already flagged it "Exists but not used". - src/hooks/use-notifications.ts — duplicate/legacy useNotifications; the live hook is src/hooks/useNotifications.ts (capital N), consumed by the real NotificationCenter. - src/components/golf/coachhelm/insights/{InsightBulkActions,InsightExportModal, InsightFiltersPanel,InsightSearchBar}.tsx — not exported from the insights/ barrel (index.ts only re-exports PlayerFocusAreas/InsightsFeed/InsightListView per its "Wave 1A" comment), zero direct importers, no next/dynamic references. - src/lib/baseball/lifting/use-live-set-sync.ts — exports useLiveSetSync, zero importers; only mentioned in docs/audits (planned-but-never-wired). Gates: typecheck clean, check-cycles clean (33 known cycles, none new), no test files reference any of these paths. Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * devibe: remove dead files — knip batch 2/2 (golf/travel legacy, soreness barrel, lift-programs) (#859) Verified dead via grep (import path + symbol name + next/dynamic scan), then git rm. - src/components/golf/travel/{ExpenseForm,ExpenseList,ExpenseSummary,index}.ts(x) — legacy pre-Fairway components. Superseded by src/components/fairway/pages/travel/ Fairway{ExpenseForm,ExpenseList,ExpenseSummary}.tsx, whose own header comments say they're re-skins of "the legacy golf/travel ExpenseList/ExpenseSummary" — i.e. the legacy files are explicitly documented as replaced. Zero live importers (grep for the barrel path and each symbol name comes back empty outside the legacy files themselves). - src/components/lifting/soreness/index.ts — barrel; zero importers (every other file in the same directory — BodySilhouetteFront, SorenessCheckCard, SorenessBodyMap, HighPrioritySorenessList, SorenessScheduleBuilder — IS imported directly by app code, just never through this barrel). - src/components/lifting/soreness/SorenessComplianceBoard.tsx, TeamSorenessHeatmap.tsx — only referenced from the dead barrel above; no direct importers. - src/lib/baseball/read-models/lift-programs.ts — exports getLiftProgramList/ getLiftProgramTree/getAssignContext. The live /performance/programs/[programId] page defines its own local getAssignContext (duplicated, not imported from here) — confirms this read-model was built but never wired in. Gates: typecheck clean, check-cycles clean (33 known cycles, none new). `grep` false-positive check: src/app/golf/actions/__tests__/travel.test.ts matches "ExpenseSummary" only via the substring in getExpenseSummary() (a server action, unrelated file) — ran that suite standalone to confirm (128 passed, 4 skipped, unaffected). Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * devibe: remove orphaned root scaffolding (.taskmaster, .full-stack-feature, stray App Store Connect snapshots) (#860) - .taskmaster/ (9 tracked files: README, config.json, docs/current-state.md, docs/feature-checklist.md, docs/prd.txt, logs/.gitkeep, state.json, tasks/tasks.json, templates/task-template.json) — task-master scaffolding from an abandoned tool integration. Only appears elsewhere as ignore-list entries (.gitignore:76-77), never read by any script/workflow/package.json script. Zero functional references. - .full-stack-feature/ (2 tracked files: 01-requirements.md, state.json) — same pattern: only appears as ignore-list entries across .gitignore, .coderabbitignore, .coderabbit.yaml, .vercelignore, .greptile/config.json, .greptile/rules.md (all just telling other tools to skip the directory). Zero functional references. - full-snapshot.yml, full-snapshot2.yml, app-info-snapshot.yml, age-ratings-snapshot.yml — accessibility-tree/DOM snapshots of the App Store Connect web UI (not fastlane config — there is no fastlane/ directory anywhere in this repo, which uses Xcode Cloud, not fastlane). Zero script or CI references (grepped scripts/, tools/, .github/, .circleci/ — nothing reads these paths). The one doc mention (docs/operations/2026-05-28-coderabbit-fails-investigation.md) explicitly calls age-ratings-snapshot.yml "INHERITED NOISE" causing ~200 yamllint indentation errors and recommends "delete it if it's truly unused" — it is. review-gate.yml's yamllint job only lints *changed* files in a PR diff, so these aren't continuously failing CI, but they're pure accidental commits (browser-automation output) with zero purpose in the repo. - context7.json — does not exist (only context7.json.example is tracked; the real context7.json was already removed in a prior commit 6a9b565 "fix(security): stop tracking context7.json (contained leaked API key)"). Nothing to do here. Gates: typecheck clean, check-cycles clean (33 known cycles, none new). Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * devibe: console triage — remove debug-leftover console.log in use-service-worker (#861) Audited the 77 console.log/debug/warn call sites in prod src (excluding tests). Two mechanisms make almost all of them deliberate, not vibe-coded leftovers, and this PR documents why nearly everything was kept: - next.config.mjs compiler.removeConsole strips console.log AND console.debug from production builds, excluding only 'error'/'warn'. So every console.log/.debug call is already dev-only/no-op in prod. - src/instrumentation.ts + src/instrumentation-client.ts both configure Sentry.consoleLoggingIntegration({ levels: ['log','warn','error'] }) — console.warn is the established, load-bearing structured-logging idiom in this codebase (forwarded to Sentry Explore → Logs), which is exactly why admin-tracer-data.ts has an explicit comment: "console.warn used (not console.log) because production build strips console.log." Reviewed every one of the 48 console.warn and 8 console.debug call sites individually: every single one has either an explicit comment justifying the log level (e.g. insight-delivery.ts's transient-fetch debug downgrade, useAdminPresence.ts's `if (process.env.NODE_ENV !== 'production')`-gated join/leave debug logs, pattern-miner.ts's documented severity policy, admin-logger.ts's PGRST205 once-only warn) or is a genuine production security/error signal (auth rate-limiting, unauthorized message/team actions, fetch-failure fallbacks). None were genuine leftovers — all kept as-is, no logger-idiom conversion performed (see below). **Deleted** (1 file, 8 statements): src/hooks/golf/use-service-worker.ts — 8 console.log calls tracing every SW lifecycle branch (register no-op, already-registered, registered, unregistered, update complete, sync unsupported, sync registered, no active worker to message, message received). Unlike every kept call site above, these had (a) no explanatory comment, (b) no dev-only guard, (c) duplicate state already exposed via the hook's own return value (`status`/`isRegistered`/ `hasUpdate`), and (d) trace literally every branch including plain early returns — the classic "log every branch while debugging a tricky SW bug" pattern (see memory: dev-SW false-offline investigation) never cleaned up. The 5 console.error calls in this same file's catch blocks are untouched (KEEP per the task rule). **Logger-idiom conversion**: grepped for a logger util first (src/lib/admin-logger.ts, server-error-logger.ts, error-logging.ts exist) — none is a general-purpose console.warn replacement; they're purpose-built for the admin_events audit trail / Sentry error classification, and console.warn already IS the repo's structured-log idiom for this class of signal (per the Sentry consoleLoggingIntegration wiring above). Converting would be redundant double-logging and risk semantic changes (async logger calls dropped into sync catch blocks) for no observability gain, so no conversions were made — warns left as-is, per the "if none, leave warns" instruction. Gates: typecheck clean, eslint --max-warnings 0 on the touched file clean, check-cycles clean (33 known cycles, none new). No test file covers this hook (grepped for use-service-worker in *.test.*/*.spec.* — zero hits). Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * Build the /baseball public marketing page (was a bare redirect) (#865) Signed-out visitors used to get bounced straight to /baseball/login with zero context; they now see a real front door — hero, four editorial feature sections (roster/team-ops, stats center, recruiting pipeline, player passport) composed from the Living Annual kit in ghost/placeholder state (no fabricated screenshots or invented player data), and an honest CTA row (Sign in / Create a program / Join with a code). Signed-in visitors keep the exact prior redirect-to-dashboard behavior. - src/app/baseball/page.tsx: rewritten from a bare redirect into the full marketing page; auth check now only fires the redirect when a session exists. - src/components/baseball/marketing/BaseballMarketingMotionScope.tsx: new tiny 'use client' LazyMotion wrapper — the Living Annual atoms used here (RuledStatLine/Masthead/HairlineRule/GradeStamp) never transition off their hidden variant without a loaded feature bundle, and the page itself stays a Server Component (async session check + redirect), so this is the one client boundary. - src/app/baseball/join/page.tsx: new — the "Join with a code" CTA needed a real destination; only the dynamic /baseball/join/[code] existed. Mirrors GolfHelm's /golf/join code-entry page, themed in the Living Annual paper/ink system instead of golf's glass-orb auth chrome. - src/components/landing/Footer.tsx: generalized the shared cross-product footer's tagline off golf-only wording ("college golf") since it now also renders under a BaseballHelm hero. - src/app/baseball/__tests__/page.test.tsx: pins the redirect/no-redirect branching (coach session, player session, signed-out). Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * Fix invisible names/numerals on public baseball profile pages (no LazyMotion ancestor) (#866) team/[id], player/[id] (via PlayerProfileClient), program/[id], and packet/[token] sit in the (public) route group, whose layout was a bare `<>{children}</>` — no LazyMotion anywhere upstream. team/[id] and PlayerProfileClient render Living Annual `m`-based atoms (Masthead, RuledStatLine, HairlineRule) directly; their `inkSettles`/`rulesDraw` entrance variants start at `hidden` (opacity: 0 / scaleX: 0) and only animate to `visible` once framer-motion's feature bundle is loaded via a `LazyMotion` ancestor. Without one, an `m.*` component's AnimationFeature never mounts, so the hidden variant is terminal for any visitor without `prefers-reduced-motion` on — player/team names and stat numerals stayed invisible on these live public recruiting pages. Adds PublicMotionScope (mirrors the existing AdminMotionProvider / `(dashboard)/dashboard/template.tsx` pattern already used elsewhere in the repo) and mounts it from `(public)/layout.tsx`, which stays a Server Component — the LazyMotion boundary lives in the client child. Verified via a real (unmocked) framer-motion render test: Masthead's surname text is measurably opacity: 0 forever with no wrapper, and measurably transitions off 0 once PublicMotionScope loads its feature bundle — the same computed-opacity check `toBeVisible()` uses, so it reproduces the actual bug and the actual fix rather than a mocked stand-in. program/[id] and packet/[token] don't currently render any Living Annual `m` atoms directly (packet's ScoutPacketView already carries its own LazyMotion) — the shared layout-level provider covers them defensively against regression as those pages grow. PR #865 (open, targets this same base) adds a near-identical BaseballMarketingMotionScope for the separate /baseball marketing root and explicitly flagged this (public) route group gap out of its own scope; this PR is the fix for that flagged gap. Not touching #865's files — noted in the PR body that the two wrappers could be consolidated into one shared component later. Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * Add production visual-audit screenshot crawl (GHA, manual-only) (#867) New e2e/visual-audit.spec.ts mirrors baseball-route-crawler.spec.ts's proven live-DOM nav discovery (FairwaySidebar + hub-sub-nav <nav> links) and best-effort public-sample-link discovery, but captures full-page screenshots at phone (390x844) and desktop (1440x900) viewports for every discovered coach/player route plus signed-out publics, instead of asserting route health. Screenshots are data capture, not assertions — the spec only fails on a login failure or a total navigation failure. Gated behind VISUAL_AUDIT=1 (test.skip otherwise); playwright.config.ts's chromium project now ignores it and baseball-coach/baseball-player now match it, so it never runs in the ordinary e2e lane and playwright.yml/ci.yml (which name their spec files explicitly) never pick it up. New .github/workflows/visual-audit.yml runs it via workflow_dispatch against a chosen base_url (default prod), --project=baseball-coach --project=baseball-player only — verified against the installed Playwright runner source that this also runs the `setup` project's full baseball auth (both roles) as a dependency, without needing an explicit --project=setup, and without ever touching Golf's auth.setup.ts. Uploads test-results/visual-audit as visual-audit-<run_number>, if: always(). Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * db(baseball): write #379 legacy stats backfill migration (pending Nick's go) (#862) * db(baseball): write #379 legacy stats backfill migration (pending Nick's go) One-time, NOT-APPLIED migration that copies legacy baseball_player_stats 'game' rows into baseball_box_score_batting/_pitching + synthesizes shared baseball_games rows, scoped to teams with ZERO existing box-score data (teams already on the box-score adapter path are never touched). Deterministic ids (SHA-1, RFC4122-v5-shaped, own namespace) mirror #827's scripts/seed-baseball-stats.mjs detId() pattern so re-applying is a no-op and rollback can recompute — not just look up — exactly which rows are ours. Copy-only: legacy rows are never mutated. Deliberately skips recalculate_baseball_season_stats() to avoid clobbering any pre-existing season_totals-imported baseline on baseball_player_season_stats — documented as an opt-in follow-up instead. Exercised end-to-end against a disposable local Postgres 16 instance (schema mirrored from the real migrations, never any shared project) covering a two-way partial-innings player, a duplicate-row collision, an already-box-score team (excluded), and a pre-existing-scheduled-game collision (date skipped) — verified idempotent re-run and a dry-run rollback recompute+delete. See docs/baseball/legacy-backfill-runbook.md for the check-first queries, apply steps, and rollback recipe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): make #379 backfill's season-stats safety story true, not just written Adversarial review on PR #862 found the migration's core safety claim false: recalculate_baseball_season_stats() is described as a deliberate, manual, opt-in, per-team step, but the already-shipped save_baseball_full_box_score RPC calls it automatically on every ordinary box-score save. Since the backfilled games carry their real historical game_date (plausibly within the current season year for teams whose whole history predates #827), the very next normal game entry for an overlapping player would silently overwrite baseball_player_season_stats -- including any pre-existing season_totals baseline -- with no opt-in and no signoff. Fix, verified against a disposable local Postgres 16 instance (never any shared Supabase project): - Migration: add Step 4, seeding baseball_player_season_stats for exactly the (player_id, team_id, season_year) triples the migration's own box-score rows touch, using the identical aggregation/rate formulas recalculate_baseball_season_stats() uses -- guarded by ON CONFLICT ... DO NOTHING so a pre-existing row (e.g. a season_totals baseline) is never touched, preserving copy-only/additive-only/idempotent. Where no row existed, the eventual live recalc now lands on the same numbers already seeded (a no-op, not a surprise). - Runbook: replace the "deliberately out of scope" framing with the true story, add a pre-flight query that surfaces exactly which triples still carry pre-existing-baseline risk (Nick must review before applying), and add a diff-based season-stats rollback procedure since DO NOTHING rows have no deterministic id to recompute against. Locally reproduced the exact scenario the review described (a fresh ordinary game save via the real, unmodified RPC): the seeded player's row extended cleanly with correct math; the pre-existing baseline player's row was silently overwritten by the (unmodified) live RPC, exactly as newly documented -- confirming the fix and the doc are both now accurate. File remains WRITE-ONLY / NOT APPLIED pending Nick's go-ahead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * baseball(engine): wire event-derived velocity into engine-run/outcome-sweep/action-baseline (#852 residual) (#864) * baseball(engine): wire event-derived velocity into engine-run/outcome-sweep/action-baseline (#852 residual) Box-score-migrated players had NO velocity metrics: their legacy exit_velocity/pitch_velocity scalar is dropped alongside superseded legacy GAME rows (engine-stat-rows.ts rule 1), and the canonical box-score tables carry no velocity columns at all. loaders.ts's eventDerived hook (#851) already threaded a per-field event-layer override into loadPlayerMetrics, but nothing called it. Adds src/lib/baseball/coachhelm/engine-event-derived.ts: a team-scoped, paginated read of baseball_pitch_events/baseball_batted_ball_events (#813 superseded-row filter) plus a pure per-player reducer that reuses elite-stat-events.ts's real buildHitterMetrics/buildPitcherMetrics + loaders.ts's eventDerivedVelocityFromMetrics -- never a second, drifting "average exit velocity" implementation. All-or-nothing degrade on read failure, mirroring engine-stat-rows.ts's own honesty rule. Wires it into all three engine callers: - engine-run.ts: full-history event pool -> loadAllPlayerMetrics. - outcome-sweep.ts: event rows filtered to the SAME per-action after-window as the box-score read, so a pre-action event never counts toward did-it-move measurement. - action-baseline.ts: full-history event pool -> the baseline capture. Tests: pure aggregation (mixed hitter/pitcher, zero-event absence, supersede filter, all-or-nothing degrade) plus per-caller wiring tests (event wins over legacy scalar for the same player; a zero-event player keeps their legacy velocity; event-read failure degrades every player to legacy). Extends stat-layer-manifest.ts's grandfathered-consumer allowlist for the new fixture files (legacy baseball_player_stats rows are the fallback pin, not staleness). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): bound velocity event read to player scope + fix sampleSize honesty (PR #864 fix-first) Two adversarial-review criticals on #864: 1. buildActionOutcomeSeed (action-baseline.ts) fired a TEAM-WIDE, unbounded, player-unscoped read of the entire pitch/batted-ball event history on every coach "convert to action" click, just to resolve ONE player's velocity scalar. loadEngineEventRows now takes an optional `playerIds` scope (`.in('pitcher_id'|'batter_id', playerIds)`, mirroring loadEngineStatRows's own `.in('player_id', playerIds)` idiom) — the single-player caller passes `[playerId]`; engine-run/outcome-sweep now pass their own already-computed roster/todo player-id lists instead of reading the whole team's history. 2. avg_exit_velocity's sampleSize was `bbCount` (every batted ball) instead of the count of rows that actually carried a non-null exit_velocity reading — inflating the honesty gate for any team whose batted-ball capture doesn't always log a radar reading. Fixed to `battedBalls.filter(b => b.exit_velocity != null).length`, and applied the same fix to the sibling avg_launch_angle metric (identical bug, same line shape). Pitcher avg_velocity was already correct. Tests: pin the DB-level player scoping (loadEngineEventRows + a buildActionOutcomeSeed integration check), and pin the sampleSize fix (10 batted balls / 4 readings -> sampleSize 4; independent launch_angle gating; hard_hit_rate's bbCount-based denominator unaffected). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * Consolidate stats-upload wizard into Import Center (canonical) (#863) * Consolidate stats-upload wizard into Import Center (canonical) Audited both wizards end-to-end (§3.11 decision: Import Center is canonical). Ported the two real capability gaps before retiring the legacy path — everything else (atomic save_baseball_full_box_score RPC, player-match corrections, dedup/provenance/rollback) was already covered by Import Center's commitImport pipeline, so nothing else needed porting: - ImportWizardClient: added a "Quick box score" entry point on the choose step (preselects game_box_score + jumps straight to Upload) plus drag-and-drop onto the dropzone and a sample-values data-preview table on the detect step — the legacy wizard's two capabilities Import Center didn't have. No server-action signatures changed. - /dashboard/stats/upload is now a pure redirect into /dashboard/import, mirroring the stats -> stats-center legacy-redirect shim idiom. Sibling error.tsx/loading.tsx removed (that idiom has neither). - Retired the now-fully-orphaned StatsUploadClient/UploadHistory components (only ever imported by the old page). - Repointed the two in-app links that still pointed at the legacy route (Command Center's "Upload stats", Stats Center's header) straight at Import Center, and dropped Stats Center's redundant "Upload" button (Import Center already sat right next to it, same destination). - Test migration: extended settings-aliases-and-legacy-redirects.test.ts with the new shim, added ImportWizardClient.quick-box-score.test.tsx for the two ported capabilities, and updated the e2e assertion that pinned the retired wizard's UI strings to assert the redirect instead. nav-registry.ts (frozen) still lists /baseball/dashboard/stats/upload in stats-center's matchPrefixes and STAFF_CAPABILITY_ROUTES/GUARD_ALLOWLIST still gate it at can_manage_stats — both harmless now (a plain redirect page, still resolves on disk, destination re-enforces can_manage_imports itself) but flagging for the orchestrator in case a follow-up wants them tidied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * Fix wizard-consolidation capability lockout + restore upload history (PR #863) Adversarial review (FIX_FIRST) flagged two criticals in the stats-upload -> Import Center consolidation: 1. CAPABILITY LOCKOUT — the /stats/upload redirect shim + the two repointed CTAs sent every viewer straight at Import Center's can_manage_imports gate, locking out every default staff role that holds can_manage_stats but not can_manage_imports (assistant/pitching/hitting/catching/defensive/strength coach — 6 of 11 canonical BASEBALL_STAFF_ROLE_PRESETS). Those roles could reach and interact with the old wizard before this consolidation. Fix: /stats/upload now branches on capability instead of redirecting unconditionally. can_manage_imports staff still forward to the full Import Center; can_manage_stats-only staff get the SAME ImportWizardClient rendered inline, restricted to the "Quick box score" entry point (new quickEntryOnly prop — skips the choose step and hides the "change data shape" affordance, no way to reach the full shape picker/event-level mode/source registry/rollback reserved for can_manage_imports staff). Middleware's STAFF_CAPABILITY_ROUTES already allowlists this exact route at can_manage_stats, so no middleware/nav-registry contract change was needed. Command Center's "Upload stats" and Stats Center's two CTAs are repointed from /dashboard/import back to /dashboard/stats/upload so every entry point resolves through the capability-aware router. 2. UPLOAD HISTORY DELETED — UploadHistory.tsx was the only surface reading baseball_stat_uploads (filename/status/processed counts); its deletion left every pre-consolidation upload record permanently unviewable. Fix: ported a read-only "Legacy uploads" section into ImportWizardClient (Living Annual idiom: Eyebrow/HairlineRule/EditorsLetter honest empty state, matching the existing "Recent imports" section), backed by getRecentUploads — an existing, already-demoSafe, already-team-scoped server action with zero prior callers. No server-action signature changes. Wired into both the full Import Center page and the new capability-aware /stats/upload entry point. Also extracted the roster-for-matching query (previously inlined in import/page.tsx) into a shared src/lib/baseball/import-roster.ts helper so both pages load player-matching data identically instead of drifting. Gates: typecheck clean, eslint --max-warnings 0 clean on all touched files, targeted + broader baseball vitest suites green (1178 tests), check-cycles clean (33 known cycles, none new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(stats-center): route import entry points by viewer capability The two Import Center entry points (header action + empty-state CTA) sent everyone through the /stats/upload shim, whose middleware gate is can_manage_stats — bouncing import-capable-but-not-stats staff (e.g. the director_ops preset) off middleware before the shim's own capability branch could forward them. The page now computes can_manage_imports server-side (same helper the shim branches on) and import-capable viewers go straight to /dashboard/import; everyone else keeps the shim path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball-import): authorize stats-only staff for box-score import commit/preview (PR #863 round-4) previewImport/commitImport were hard-gated to can_manage_imports unconditionally, so the quickEntryOnly inline wizard at /stats/upload (rendered for the 6 can_manage_stats-only staff presets) let a stats-only coach fill out the whole form and then fail server-side on submit. Pre-consolidation, stats-only staff could upload box scores via the legacy wizard, so restore that: a 'game_box_score' request may now be authorized by can_manage_imports OR can_manage_stats; every other shape (season_totals, event_log, or omitted) keeps the original can_manage_imports-only gate. - with-baseball-action.ts: requiredCapability now also accepts a readonly array (ANY-of) or a resolver function of the action's own args, resolved once before AUTH so tags/metadata and enforcement can never disagree. Single-capability call sites (~60 existing) resolve to a one-element list and behave byte-identically to before. - imports.ts: previewImport gained an optional dataShape field (mirroring CommitImportArgs.dataShape) so the same shape-conditional gate applies at preview time too; both actions resolve the OR-gate from the exact field applyImportPlan uses for canonical-table routing, so the auth decision and the write decision can never diverge. - ImportWizardClient.tsx: pass dataShape through to previewImport, and hide the Upload step's "Back to choose" button for quickEntryOnly viewers (it routed to the full shape picker Import Center reserves for can_manage_imports staff). - New suite (imports-capability-shape-gate.test.ts) exercises the real withBaseballAction/capabilities wiring (not a passthrough mock) to prove: stats-only + game_box_score authorizes and actually writes; stats-only + season_totals still throws BaseballCapabilityError with zero side effects; no-capability staff still denied; imports-only staff unchanged across every shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa --------- Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * ci(visual-audit): two spaces before inline version comments (yamllint strict) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(migration): qualify digest() as extensions.digest — pgcrypto is not in public The 42883 failure reproduced on the CI fresh-stack replay and would have occurred identically on prod at apply time: pgcrypto lives in the extensions schema in both environments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * db(baseball): manifest-based rollback + concurrency lock for #379 backfill (CodeRabbit #868) - Copy-only summary now lists Step 4's baseball_player_season_stats write (finding 1). - Add permanent, service-role-only baseball_legacy_backfill_manifest ledger (RLS enabled, anon/authenticated revoked); every Step 1-4 INSERT records its own RETURNING rows into it, same transaction, tagged with a run_tag. Rollback now joins against the manifest instead of recomputing deterministic ids from current (possibly-changed) baseball_player_stats, and the runbook's rollback + season-stats-rollback sections are rewritten around manifest-join DELETEs. Verified recalculate_baseball_season_stats() does a full from-scratch rebuild (not an incremental merge) before writing the "safe to delete" rollback caveat (finding 2). - Take an explicit LOCK TABLE ... IN SHARE ROW EXCLUSIVE MODE on all 5 read/written tables before the eligibility snapshot; runbook gains an apply- window note. Confirmed SHARE ROW EXCLUSIVE cannot self-conflict with this migration's own later INSERTs (finding 9). - Rename the two TEMP TABLEs to the required baseball_ prefix, all references (finding 10). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): validate invite code is alphanumeric before router.push (CodeRabbit #868) The hint text promises "letters and numbers" but only length was checked, letting URI-breaking characters (?, #, /) reach router.push(`/baseball/join/${trimmed}`). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): suppressHydrationWarning on legacy-upload created_at cell (CodeRabbit #868) toLocaleDateString() formats with the server's locale/timezone during SSR but the browser's on hydration, risking a mismatch warning. Matches this repo's existing suppressHydrationWarning-on-the-enclosing-element precedent (LocalTime.tsx, RelativeTime.tsx, Fairway calendar/announcements components). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): hide Stats Center import actions for staff with neither capability (CodeRabbit #868) canManageImports=false conflated stats-capable staff (routed through the /stats/upload shim) with staff holding NEITHER can_manage_imports nor can_manage_stats, whom both routes would just bounce off their own middleware gate. page.tsx now Promise.all's a second hasBaseballCapability call for can_manage_stats and passes both down; StatsCenterClient renders the header "Import Center" action and the empty-state "Import a box score" CTA only when canManageImports || canManageStats holds, keeping the existing importEntryHref branch for the visible cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): filter provenance to the reading-bearing rows sampleSize counts (CodeRabbit #868) avg_exit_velocity/avg_launch_angle (hitting) and avg_velocity (pitching) each correctly narrow sampleSize to rows with an actual non-null reading, but still passed the FULL bbProv/pProv array (every batted ball / pitch, hand-charted or radar-read) into dominantTrust/dominantContext. A majority of hand-charted, no-reading rows could drag trustTier down to 'unverified' even when every row that fed the average was 'official' radar data. Pass the same `.filter(reading != null)` array as provenance in all three call sites. Extends the #864 sampleSize-honesty suite with mixed-trust regression tests (few official radar rows + many unverified hand-charted rows -> trustTier must reflect only the radar rows) for the batting and pitching paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa * fix(baseball): resolve capability requirement inside guarded flow + reject empty results (CodeRabbit #868) Two related fixes to withBaseballAction: - The (possibly args-conditional) requiredCapability resolver ran BEFORE Sentry.withScope/the wrapper's own try/catch even started, so a throwing resolver (e.g. a malformed/omitted argument) threw raw and unsanitized, skipping AUTH, Sentry, and logServerException entirely. Resolution now happens inside the guarded try/catch, right after AUTH resolves and before capability enforcement — a throwing resolver now produces the same sanitized BaseballActionError + Sentry-logged path as any other action failure. Still resolved exactly once, from the same args reference; tags/ breadcrumbs are set from the resolved value immediately afterward. - requiredCapability's array forms are now typed as non-empty tuples (readonly [BaseballCapability, ...BaseballCapability[]]) so `[]` is a compile-time error, and a resolver that manufactures an empty array at runtime anyway is rejected with a thrown BaseballCapabilityError (fail closed) instead of falling through to `resolvedCapabilityList[-1]` === undefined being passed to requireBaseballCapability. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa --------- Co-authored-by: Fable Integrator <fable@helm.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Task
379-engine-4b— implements the "CoachHelm engine Phase 4b — generators, effectiveness, outcome-sweep, rule engine" chunk of the #379 stats-layer reconciliation design (design-379.json), plus the #811 residual (deterministic clock forimportQualityGenerator). Builds directly on #827 (Phase-0 seed), #828 (sharedlegacy-stat-adapters.ts), and #851 (Phase 4anormalizeBoxScoreBattingRow/normalizeBoxScorePitchingRow+eventDerivedgroundwork), all already onbatch/bbh-finish-0714.Per-change notes
Reader migration (the real data-flow change)
src/lib/baseball/coachhelm/engine-stat-rows.ts— the ONE consolidated per-session stat-row read for the engine. Canonicalbaseball_box_score_batting/_pitchingrows (normalized via fix(baseball): CoachHelm engine loaders/registry — #379 Phase 4a #851's helpers,session_datejoined frombaseball_games, source-table provenance tags) reconciled over legacybaseball_player_statsrows per the design precedence rule:stat_type='game'rows leave the pool, never blended (the fix(baseball): reconcile seed stats with Stats Center + drift/smoke tests (#379 Phase 0) #827 seed writes the same games into BOTH layers; blending would double count);Canonical-side read failures degrade all-or-nothing to the legacy pool (a partial batting-only blend could double count a two-way player); legacy read failure stays the callers' hard error. Every read paginates past the PostgREST 1000-row cap with stable ordering (
fetchAllRowsResultidiom).Documented caveat: legacy GAME-row
exit_velocity/pitch_velocityscalars drop with the row for box-score players — box-score tables carry no velocity; the canonical velocity source is the elite event layer (fix(baseball): CoachHelm engine loaders/registry — #379 Phase 4a #851eventDerived), per design rule 4.outcome-sweep.ts/action-baseline.ts/engine-run.ts— all three swap their directbaseball_player_statsreads for the shared helper, so baseline capture, the sweep's after-window, and the engine run measure the SAME reconciled pool (apples-to-apples did-it-move).action-baseline's old single-page.limit(1000)read is now paginated via the shared read.#811 residual (deterministic engine clock)
BaseballV10EngineInputsgains optionalnow(ISO);generateAllBaseballCandidatesthreads it toimportQualityGenerator(runs, nowIso = new Date().toISOString()), whose 14-day cutoff now computes fromnowIsoinstead of rawDate.now(). Default preserves real-time behavior for non-engine callers;runBaseballEngineCorepasses its ownnowIso.generators/v10.test.tspins the window against a fixed 2020 clock (would fail on any regression toDate.now());engine-run-helm-lifting.test.tsextended to pinengineInputs.now === nowIsoend-to-end through the real run.Label-only files (per design: "no data-flow change")
generators/index.ts—driver()'s dead-code fallback label no longer hardcodes the deprecated table (loaders since fix(baseball): CoachHelm engine loaders/registry — #379 Phase 4a #851 cite the real per-row table); neutral'box score'instead → file leaves the manifest.generators/v10.ts— practice-effectiveness cite deliberately stays onbaseball_player_statswith an explanatory comment: its feeder (actions/practice-effectiveness.ts) still assembles MeasurementPoints from legacy practice rows; re-pointing the cite would be dishonest provenance.effectiveness/engine.ts+operational-rule-engine.ts— reviewed, deliberately not code-changed (their cites are honest while their feeders still read layer 1: practice-effectiveness's legacy read, and operational-signals' recent-window read which PR fix(baseball): #379 Phase 2 — reconcile operational-signals cold-streak baseline through shared adapter #847 keeps on layer 1). Resolution recorded as precise manifest notes instead of speculative provenance plumbing with no consumer.ai-policy-enforcement.test.ts/signal-from-insight.test.ts— fixture source refs moved to canonical table names (mirroring production post-fix(baseball): CoachHelm engine loaders/registry — #379 Phase 4a #851/4b provenance) → both leave the manifest.Manifest (contract kept green in both directions for this chunk's files)
Removed 6 migrated entries; added
engine-stat-rows.ts+ its test (the design's "one place allowed" legacy-fallback read, pending Phase 5); updated notes forloaders.ts,v10.ts,effectiveness/engine.ts,operational-rule-engine.ts, and the three engine test entries (now pinning the fallback tier by design, not staleness).Gates (no pipe-masking; exit codes captured explicitly)
npm run typecheck→ exit 0.npx eslint --max-warnings 0 <all 15 touched files>→ exit 0.engine-run-*suite +engine-v10.test.ts): 106/106 passing.npm run test:run(unit): 5111 passed / 8 failed — all 8 insrc/components/fairway/pages/recruiting/FairwayRecruitingPage.test.tsx, A/B-proven pre-existing: identical 8 failures on a pristine detached worktree at base0056bc0e(scratch-worktree A/B, no stash). Zero failures in any file this PR touches.npm run test:business: 5184 passed / 10 failed — the same 8 Fairway failures + the 2stat-layer-contract.test.tsfailures already documented as pre-existing in fix(baseball): CoachHelm engine loaders/registry — #379 Phase 4a #851's PR notes: offenderspractice-effectiveness.test.ts/player-today-self-scope.test.ts/player-today-honest-loop.test.ts(owned by fix(baseball): migrate player-today/passport/snapshot-cards off legacy stat layer (#379) #845) and stale entryinsights.ts(removed by fix(baseball): #379 Phase 2 — reconcile operational-signals cold-streak baseline through shared adapter #847). None of this PR's files appear in either failure list — this PR adds zero new offenders and zero new stale entries.Coordination notes
nowfield, v10.ts nowIso param, v10.test.ts, helm-lifting now-pin) — adopted per coordinator direction; gated together with the reader migration above.🤖 Generated with Claude Code
https://claude.ai/code/session_01H9QAYqFTKsXGsVw6wXYssa